chore: Add options to useEndpoint
#37020
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughcallEndpoint now accepts an optional AbortSignal and this signal is threaded from hooks and client providers into endpoint calls; useEndpoint and a query hook forward the signal, and ServerProvider passes it into REST SDK calls for cancellation support. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as UI Component
participant H as useEndpoint / useQuery
participant SC as ServerContext.callEndpoint
participant SP as ServerProvider
participant SDK as REST SDK / Endpoint Handler
UI->>H: invoke endpoint(args, options{signal?}) / start query
H->>SC: callEndpoint({...args, signal: options?.signal / signal from query})
SC->>SP: delegate request(method, path, params, signal)
SP->>SDK: sdk.rest.<method>(..., { signal })
SDK-->>SP: response / abort
alt Abort signaled
SDK-->>SP: AbortError
SP-->>SC: propagate AbortError
SC-->>H: Promise rejected (AbortError)
H-->>UI: propagate rejection / cancel
else Completed
SDK-->>SP: result
SP-->>SC: serialized result
SC-->>H: Promise<Serialized<Result>>
H-->>UI: deliver result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37020 +/- ##
===========================================
+ Coverage 67.37% 67.39% +0.02%
===========================================
Files 3330 3330
Lines 113482 113482
Branches 20598 20607 +9
===========================================
+ Hits 76454 76480 +26
+ Misses 34418 34394 -24
+ Partials 2610 2608 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
tassoevan
force-pushed
the
chore/useEndpoint-abortSignal
branch
from
d9a26d5 to
0a85bbd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/client/providers/ServerProvider.tsx (1)
61-61: Optional: support cancellation for uploads, too.If
sdk.rest.postaccepts{ signal }, consider threading it here in a follow‑up to keep behavior consistent.-const uploadToEndpoint = (endpoint: PathFor<'POST'>, formData: any): Promise<UploadResult> => sdk.rest.post(endpoint as any, formData); +const uploadToEndpoint = ( + endpoint: PathFor<'POST'>, + formData: any, + options?: { signal?: AbortSignal }, +): Promise<UploadResult> => sdk.rest.post(endpoint as any, formData, options);Note: This changes the public signature; update
ServerContextValueaccordingly if adopted.apps/meteor/client/apps/gameCenter/hooks/useExternalComponentsQuery.ts (1)
9-11: LGTM: query cancellation is correctly wired.Forwarding React Query’s
{ signal }intouseEndpointenables proper aborts.You could use
selectto project the field and keep the fetcher simpler:- queryFn: async ({ signal }) => { - return (await getExternalComponents(undefined, { signal })).externalComponents; - }, + queryFn: ({ signal }) => getExternalComponents(undefined, { signal }), + select: (data) => data.externalComponents,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/meteor/client/apps/gameCenter/hooks/useExternalComponentsQuery.ts(1 hunks)apps/meteor/client/providers/ServerProvider.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/meteor/client/providers/ServerProvider.tsx (2)
packages/rest-typings/src/index.ts (3)
UrlParams(202-208)OperationParams(194-196)OperationResult(198-200)apps/meteor/app/utils/client/lib/SDKClient.ts (1)
sdk(271-271)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/client/providers/ServerProvider.tsx (1)
28-40: AbortSignal threading looks good; verify upstream type surface matches.Sandbox lacked repository files — run these locally to confirm ServerContextValue and useEndpoint typings expose
signal?: AbortSignal(and that callers don't fall back toany):rg -nP -C2 --type=ts 'callEndpoint\s*:\s*<[^>]+>\s*\(\s*\{[^}]*signal\?:\s*AbortSignal' packages/ui-contexts/src/ServerContext.ts rg -nP -C2 --type=ts 'type\s+EndpointOptions\s*=\s*\{\s*signal\?:\s*AbortSignal' packages/ui-contexts/src/hooks/useEndpoint.ts
tassoevan
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Co-authored-by: Tasso Evangelista <2263066+tassoevan@users.noreply.github.com>
Co-authored-by: Tasso Evangelista <2263066+tassoevan@users.noreply.github.com>
Co-authored-by: Tasso Evangelista <2263066+tassoevan@users.noreply.github.com>
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit